Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

uid for each connected client #96

Merged
merged 2 commits into from
Sep 5, 2024
Merged

Conversation

henriquetft
Copy link
Contributor

closes #66

Please check if the implementation matches what you had in mind. If it does, I'll proceed and update the README and the man pages.

Copy link
Owner

@Theldus Theldus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @henriquetft,
Your PR is exactly how I envisioned a UID implementation, thank you very much for your work on this.

Since you said you will also update the README.md and man-pages, I will wait before merging this, but it is already approved =).

@henriquetft
Copy link
Contributor Author

I've just pushed another commit to update the README and manpages. Let me know if anything else needs adjustment!

Copy link
Owner

@Theldus Theldus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These changes look great to me too, and thanks again for working on issue #66.

@Theldus Theldus merged commit ea8a029 into Theldus:master Sep 5, 2024
3 checks passed
Theldus added a commit that referenced this pull request Sep 16, 2024
The bug was introduced in PR #96, which added support for a client UID.

The issue is that by adding a "UID," it also necessitates looping through
the list of clients to find the connection corresponding to the specific
UID. This loop needs to be protected by a mutex to prevent modifications
to the client list while it's being traversed, and this part works
perfectly fine.

The problem arises when a ping broadcast is sent to all active clients.
In that case, the client list is also locked with the same mutex, causing
the code to enter a blocking state.

The fix is quite simple: switch from using `ws_sendframe()` to
`ws_sendframe_internal()`, since the latter does not rely on the client's
UID but instead uses the connection structure, which was already obtained
earlier. This way, no additional locks are needed, and the code runs
without issues.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connection UID
2 participants